Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GitHub Action to test the code on Linux, macOS, and Windows #264

Merged
merged 14 commits into from
Mar 27, 2024

Conversation

cclauss
Copy link

@cclauss cclauss commented Jul 24, 2023

@cclauss cclauss changed the title Add GitHub Action to test the code Add GitHub Action to test the code on Linux, macOS, and Windows Jul 24, 2023
@cclauss
Copy link
Author

cclauss commented Mar 26, 2024

@dhritzkiv Your review, please. It would be great to have automated testing.

@dhritzkiv
Copy link
Member

dhritzkiv commented Mar 26, 2024

Thank you for taking this on! It will be great to have CI in one, hassle-free place.

A few questions:

  • Test failures
    • Any idea about those latest failures? It seems the tests simply stop before completion, especially in the latest commit.
    • Would re-running the builds resolve this?
  • Is there a reason why ubuntu-latest is excluded?
  • one functionality that's missing (and is arguably most important) is building prebuilt images and uploading them to GitHub as part of the release, in order to skip building from source and speed up installation with npm
    • Basically, if tests pass OK, and the commit tag == version of the package, a script is run to build prebuilt binaries for linux/linux-musl, macOS (arm and intel), and Windows 64 bit.
    • I'm completely new to GitHub Actions, and while I could likely figure out the steps involved, it would be helpful if you could set that up as a workflow (I think that's the term).
    • Refer to after_success.sh for what I'm talking about. I can also take a stab at it, though it'll likely take more time.
    • Publishing packages to GitHub might be a good alternative/ a more correct way to address this.
    • Publishing to npm could also be achieve from GitHub actions. Perhaps these should be combined?

@dhritzkiv
Copy link
Member

As for angle upgrades, I'd keep it separate from this PR.

@cclauss
Copy link
Author

cclauss commented Mar 26, 2024

Please merge this so we have tests in place. Then in future PRs we can add more OSes, Node versions, builds, and releases.

@dhritzkiv
Copy link
Member

I intend to.

This is a good start, but I'd love to see the tests passing primarily on Node v20 on Linux and Windows first. (Node v16 or v18, as well as macOS tests would be a bonus, but are not necessary).

Right now, this CI configuration, as the majority of images are excluded/failing, doesn't offer much over Travis CI. Granted, this configuration does works better than Appveyor for Windows tests (as that CI environment has been non-functional for us for the last 6 months), so I suppose this PR is a net-positive in that regard.

I'll concede that prebuilds/packaging/releasing can be added later.

Ultimately, I don't see a point merging this in a half-working state, unless I'm missing something?

@cclauss
Copy link
Author

cclauss commented Mar 26, 2024

Do you pay for Travis CI or are you working on free credits?

It seems lik 10386 no plan found is the test that fails on Appveyor as well as GHAs. I do not have the expertise to fix that test.

As said above, it is much easier to enable tests after the guardrails are in place.

@dhritzkiv
Copy link
Member

Do you pay for Travis CI or are you working on free credits?

Working on free OSS credits.

@cclauss
Copy link
Author

cclauss commented Mar 26, 2024

Travis credits will run out really fast.

@dhritzkiv
Copy link
Member

Yes, that's why I limited Travis to run only on the latest two LTS version of Node on Ubuntu. No other versions of Node, and no macOS.

Once in a while, we run out of credits, but reaching out to Travis helps to replenish them. Not ideal, but not the worst.

Migrating to GitHub actions will help avoid this, for sure.

@dhritzkiv
Copy link
Member

Many years ago, we used to get intermittent no plan found errors in our tests (both in Travis, AppVeyor, and locally), and almost always the workaround was to re-run the tests.

Eventually, these stopped happening entirely, whether due to using newer versions of Node, or updated test-running dependencies. Everything was good for about 4 years. Unfortunately, it seems these errors are back. :/

It looks like your contribution in #280 failed the first time, but succeeded on a subsequent run. This might suggest that re-running GitHub actions until they succeed might be the temporary answer.

@dhritzkiv
Copy link
Member

dhritzkiv commented Mar 26, 2024

Is it possible to force a Github action workflow run to be re-run if it failed? Doing so manually would be fine.

@cclauss
Copy link
Author

cclauss commented Mar 26, 2024

Re-run failed joobs

@dhritzkiv
Copy link
Member

Okay, I see that these tests are flaky, and has nothing to do with the configuration/environment of the GitHub Actions specifically.

I see your latest commits restore the other OSes, and they are surprisingly passing.

I will merge this MR later this evening.

@cclauss
Copy link
Author

cclauss commented Mar 26, 2024

All passed at https://github.com/cclauss/headless-gl/actions

https://github.com/coactions/setup-xvfb was the key to Ubuntu.

@dhritzkiv dhritzkiv merged commit fefd240 into stackgl:master Mar 27, 2024
1 of 2 checks passed
@cclauss cclauss deleted the patch-1 branch March 27, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants